Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: export emails #591

Merged
merged 24 commits into from
Nov 30, 2020
Merged

feat: export emails #591

merged 24 commits into from
Nov 30, 2020

Conversation

no-or
Copy link
Contributor

@no-or no-or commented Nov 6, 2020 •
edited by chuck-sys
Loading

Ticket(s)

Closes #539

Details

/rocket export emails for exporting emails of all users + list of all the users who don't have an email

Example run of the command:

Screen Shot 2020-11-05 at 18 40 43 PM

@no-or no-or requested review from a team as code owners November 6, 2020 02:56
@codecov
Copy link

codecov bot commented Nov 6, 2020 •
edited
Loading

Codecov Report

Merging #591 (d3f6c54) into master (eb95e33) will decrease coverage by 0.42%.
The diff coverage is 81.90%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #591      +/-   ##
==========================================
- Coverage   93.09%   92.67%   -0.43%     
==========================================
  Files          42       43       +1     
  Lines        2420     2524     +104     
  Branches      323      337      +14     
==========================================
+ Hits         2253     2339      +86     
- Misses        109      123      +14     
- Partials       58       62       +4
Impacted Files Coverage Δ
tests/util.py 100.00% <ø> (ø)
app/controller/command/commands/export.py 81.18% <81.18%> (ø)
app/controller/command/commands/__init__.py 100.00% <100.00%> (ø)
app/controller/command/parser.py 94.59% <100.00%> (+0.07%) ⬆️
db/utils.py 75.00% <0.00%> (+4.16%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update eb95e33...d3f6c54. Read the comment docs.

chuck-sys
chuck-sys requested changes Nov 6, 2020
View reviewed changes
Copy link
Collaborator

@chuck-sys chuck-sys left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just got off a meeting (that I couldn't do a code review in, boohoo). But now that I'm out....

(Great job btw)

app/controller/command/commands/export.py Outdated Show resolved Hide resolved
app/controller/command/commands/export.py Outdated Show resolved Hide resolved
app/controller/command/commands/export.py Outdated Show resolved Hide resolved
app/controller/command/commands/export.py Outdated Show resolved Hide resolved
app/controller/command/commands/export.py Outdated Show resolved Hide resolved
@bobheadxi bobheadxi changed the title Export emails completed feat: export emails Nov 7, 2020
chuck-sys
chuck-sys reviewed Nov 8, 2020
View reviewed changes
app/controller/command/commands/export.py Outdated Show resolved Hide resolved
app/controller/command/commands/export.py Outdated Show resolved Hide resolved
app/controller/command/commands/export.py Outdated Show resolved Hide resolved
chuck-sys
chuck-sys requested changes Nov 28, 2020
View reviewed changes
Copy link
Collaborator

@chuck-sys chuck-sys left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some things.

app/controller/command/commands/export.py Outdated Show resolved Hide resolved
app/controller/command/commands/export.py Outdated Show resolved Hide resolved
app/controller/command/commands/export.py Outdated Show resolved Hide resolved
app/controller/command/commands/export.py Show resolved Hide resolved
chuck-sys
chuck-sys requested changes Nov 29, 2020
View reviewed changes
Copy link
Collaborator

@chuck-sys chuck-sys left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. May need some tests for handle_char_limit_exceeded(). Other than that only some minor things in docs.

app/controller/command/commands/export.py Outdated Show resolved Hide resolved
app/controller/command/commands/export.py Outdated Show resolved Hide resolved
app/controller/command/commands/export.py Show resolved Hide resolved
app/controller/command/commands/export.py Outdated Show resolved Hide resolved
@chuck-sys chuck-sys merged commit fb993b8 into master Nov 30, 2020
@chuck-sys chuck-sys deleted the export-emails branch November 30, 2020 06:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bobheadxi bobheadxi bobheadxi left review comments

@chuck-sys chuck-sys chuck-sys approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

command to export emails of subsets of users
3 participants
@no-or @chuck-sys @bobheadxi